Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix white space around top and bottom edges of extra long images #6384

Merged
merged 2 commits into from
Nov 30, 2021

Conversation

aswin-s
Copy link
Contributor

@aswin-s aswin-s commented Nov 21, 2021

Details

This PR fixes an issue where a white empty space was shown at the top and bottom of extra long images.

There was a 1px border around the thumbnail image which was not taken into account while calculating the image dimensions. This resulted in the rendered image from getting squished due to box-sizing. To fix this thumbnail styles are now applied to a parent View which wraps the fixed width container that embeds the image.

Fixed Issues

$ #5162

Tests

QA Steps

  1. Open chat
  2. Upload extra long image Please use Sample1 and Sample2 image
  3. Once image uploaded, verify that there is no extra white space around the top and bottom edges of the extra long image.
  4. Verify that existing styles for thumbnail images are applied as expected.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screenshot 2021-11-21 at 9 38 32 AM

Mobile Web

localhost_8080_r_83562329

Desktop

Screenshot 2021-11-21 at 9 28 58 AM

iOS

iOS

Android

Screenshot_2021-11-21-09-31-58-143_com expensify chat

@aswin-s aswin-s requested a review from a team as a code owner November 21, 2021 04:14
@MelvinBot MelvinBot requested review from tgolen and removed request for a team November 21, 2021 04:14
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@aswin-s
Copy link
Contributor Author

aswin-s commented Nov 21, 2021

I have read the CLA Document and I hereby sign the CLA

@aswin-s
Copy link
Contributor Author

aswin-s commented Nov 23, 2021

@tgolen Gentle reminder.

Looks like the workflow requires an approval to proceed as this is my first PR here.

Copy link
Contributor

@tgolen tgolen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay! I was away on vacation.

@tgolen tgolen merged commit e02156c into Expensify:main Nov 30, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@aswin-s aswin-s deleted the aswin-s/issue_5162 branch November 30, 2021 15:44
@OSBotify
Copy link
Contributor

OSBotify commented Dec 7, 2021

🚀 Deployed to staging by @tgolen in version: 1.1.17-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Dec 8, 2021

🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants